Speech transcribe async bug by ryanmats · Pull Request #688 · GoogleCloudPlatform/python-docs-samples

ryanmats · 2016-11-29T03:39:33Z

Fixed bugs for Speech API GRPC sample code:

Issues with import statements
Changed default encoding choice from FLAC to LINEAR16. Added code to print error message if there was an error with the transcribe_async long running operation. (As per Jerjou's suggestion)
Added tests for GRPC transcribe and transcribe_async samples. Code uploads the audio.raw blob to the default CLOUD_STORAGE_BUCKET, tests sample code using this blob's URI, and deletes the blob at the end.

…n-docs-samples into speech-transcribe-async-bug

theacodes · 2016-11-29T17:51:23Z

@ryanmats all of the speech test resources should be pre-uploaded, so there should be no need to upload/delete the blob every time (see scripts/prepare-testing-project.sh L47).

theacodes · 2016-11-29T17:52:09Z

Other than the note about the tests, this LGTM. @jerjou can you take a quick look before we merge?

jerjou · 2016-11-29T19:01:01Z

speech/grpc/transcribe_async.py

+        if operation.error.message:
+            print("")
+            print("Operation error:")
+            print(operation.error)


What? Jon didn't wasn't like, "Please use single quotes for strings"?? WHO ARE YOU AND WHAT HAVE YOU DONE TO @jonparrott ?!? :)

I'd just combine this into one string, for brevity:

print('\nOperation error:\n{}'.format(operation.error))

We should be paid proportional to like, the inverse of lines of code :) Except then we'd be incentivized to not write any code at all... hm...

I blame my lack of sleep.

jerjou · 2016-11-29T19:03:56Z

speech/grpc/transcribe_async_test.py

+    assert re.search(r'how old is the Brooklyn Bridge', out, re.DOTALL | re.I)
+
+    # Delete audio file from storage bucket
+    blob.delete()


+1 just have the asset on the GCS bucket already :)

But for future reference, if you're going to do this, should probably put this in a context manager (so that eg if the assertion fails, the blob is still deleted), or in one of those fancy pytest fixtures (same idea)

jerjou · 2016-11-29T19:05:56Z

Minor comments. Mainly just Jon's thing :) Otherwise, looks good.

…n-docs-samples into speech-transcribe-async-bug

ryanmats · 2016-11-29T21:56:55Z

@jonparrott @jerjou I pushed the changes based on your code review comments. Also updated the speech sample audio filename in scripts/prepare-testing-project.sh.

theacodes · 2016-11-29T22:00:06Z

scripts/prepare-testing-project.sh


 echo "Creating speech resources."
-gsutil cp speech/api/resources/audio.flac gs://$GCLOUD_PROJECT/speech/
+gsutil cp speech/api-client/resources/audio.raw gs://$GCLOUD_PROJECT/speech/


Are there other tests that depend on audio.flac?

Just searched through the repo and it doesn't look like there are. Also, the audio.flac file doesn't exist in that directory anymore.

Might want to set -e on the top of this file, so that the build will fail in the future when this happens..

(doesn't have to be in this CL - just saying it'd probably be a good idea..)

changed in this PR

theacodes · 2016-11-29T22:00:34Z

speech/grpc/transcribe_async_test.py

+def test_main(resource, capsys):
+
+    # Run the transcribe_async sample on audio.raw, verify correct results
+    speech_storage_uri = 'gs://' + os.environ.get('CLOUD_STORAGE_BUCKET')


Use the cloud_config or remote_resource fixture.

…ments

theacodes · 2016-11-30T20:24:08Z

conftest.py

 @pytest.fixture(scope='session')
 def cloud_config():
    """Provides a configuration object as a proxy to environment variables."""
+    speech_storage_uri = 'gs://' + os.environ.get('CLOUD_STORAGE_BUCKET')


@ryanmats there's already the storage_bucket thing here, there isn't any need to add another one.

…n-docs-samples into speech-transcribe-async-bug

theacodes · 2016-11-30T20:54:29Z

speech/grpc/transcribe_async_test.py

+def test_main(resource, capsys, cloud_config):
+
+    # Run the transcribe sample on audio.raw, verify correct results
+    speech_storage_uri = 'gs://' + cloud_config.storage_bucket


Lol okay, I promise I'm not doing this just to annoy you:

Prefer:

storage_uri = 'gs://{}/speech/audio.raw'.format(cloud_config.storage_bucket)

(Here and in the other test file).

) Source-Link: googleapis/synthtool@0c7b033 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:08e34975760f002746b1d8c86fdc90660be45945ee6d9db914d1508acdf9a547 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

ryanmats added 3 commits November 28, 2016 18:28

Fixed Speech API GRPC sample import issues, added tests

8a7355e

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

0a3878e

…n-docs-samples into speech-transcribe-async-bug

Revised tests for speech grpc samples to upload sample audio and use it

e4c1f71

ryanmats assigned theacodes Nov 29, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 29, 2016

ryanmats assigned jerjou and puneith Nov 29, 2016

jerjou reviewed Nov 29, 2016

View reviewed changes

ryanmats added 2 commits November 29, 2016 13:53

Made changes based on Jon/Jerjou's code review

695cd78

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

d3fee87

…n-docs-samples into speech-transcribe-async-bug

theacodes reviewed Nov 29, 2016

View reviewed changes

ryanmats added 2 commits November 29, 2016 15:44

Changed tests to use cloud_config for speech_storage_uri

111eb09

Added set -e to prepare-testing-project.sh script as per Jerjou's com…

6f6134b

…ments

theacodes reviewed Nov 30, 2016

View reviewed changes

ryanmats added 3 commits November 30, 2016 12:31

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

fb9d2b6

…n-docs-samples into speech-transcribe-async-bug

Modified cloud_config for speech grpc tests

bcbe0ab

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

2322792

…n-docs-samples into speech-transcribe-async-bug

theacodes reviewed Nov 30, 2016

View reviewed changes

Fixed formatting issues with storage_uri string

01a1603

theacodes merged commit ff54b24 into master Nov 30, 2016

theacodes deleted the speech-transcribe-async-bug branch November 30, 2016 21:18

Linchin mentioned this pull request Aug 18, 2025

chore: migrate handwritten samples from googleapis/python-bigquery-storage #13543

Merged

9 tasks

Conversation

ryanmats commented Nov 29, 2016

Uh oh!

theacodes commented Nov 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes commented Nov 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerjou commented Nov 29, 2016

Uh oh!

ryanmats commented Nov 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

theacodes commented Nov 29, 2016 •

edited

Loading